Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recalculate listeners' best block heights on each retry #471

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Feb 13, 2025

Previously, we would calculate each listener's best block height outside of the loop, which will have us retrying init::synchronize_listeners with stale data, potentially leading to panicking as we might try to connect blocks from the original height to listeners that might have been advanced by the previous itertation of the loop before hitting an error. Here, we mitigate this by simply retrieving the current best block height for each listener at the start of each iteration.

Previously, we would calculate each listener's best block height outside
of the `loop`, which will have us retrying `init::synchronize_listeners`
with stale data, potentially leading to panicking as we might try to
connect blocks from the original height to listeners that might have
been advanced by the previous itertation of the `loop` before hitting an
error. Here, we mitigate this by simply retrieving the current best
block height for each listener at the start of each iteration.
@tnull tnull requested a review from jkczyz February 13, 2025 12:42
@tnull tnull added this to the 0.5 milestone Feb 13, 2025
Comment on lines +296 to +306
let mut chain_listeners = vec![
(
onchain_wallet_best_block_hash,
&**onchain_wallet as &(dyn Listen + Send + Sync),
),
(
channel_manager_best_block_hash,
&*channel_manager as &(dyn Listen + Send + Sync),
),
(sweeper_best_block_hash, &*output_sweeper as &(dyn Listen + Send + Sync)),
];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline that making current_best_block a method on Listen would be useful. It simplifies the API and would avoid needing to allocate a Vec on each iteration.

@tnull tnull merged commit 6de3500 into lightningdevkit:main Feb 13, 2025
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants